Repository navigation
ldaps: add LDAPS support - #264
shridhargadekar wants to merge 8 commits into
Conversation
shridhargadekar
commented
Jul 31, 2026
- Add export_root_ca_certificate() to ADHost, SambaHost, and IPAHost
- Add CertUtils (client.cert) utility for system-level CA cert install into /etc/openldap/ldap.conf — works with adcli, realmd, ldapsearch
- Add SSSDCommonConfiguration helpers: ad_use_ldaps(), samba_use_ldaps(), ipa_set_tls_cacert() for SSSD-specific LDAPS configuration
74dde9f to
3721fa4
Compare
a94e0cc to
41ed743
Compare
spoore1
left a comment
There was a problem hiding this comment.
Just a couple of quick notes until I can test this out.
41ed743 to
0cc8a13
Compare
|
Depends upon #255 |
e2b0d8a to
7b80a98
Compare
spoore1
left a comment
There was a problem hiding this comment.
I'm not sure the AD export method is working as expected. My testing showed the certutil fail and the old commit worked better but, I'm not sure it's pulling the expected certificate.
effb2c3 to
b9de358
Compare
|
Hi, it looks like Do I see it correctly that with the given approach bye, |
AD had to be intentionally skipped as there is limitation in this approach of using The bare minimal fix will be to move For now, AD tests can still install the cert per-test via
Yes, these changes are additional cost/complexity for any project like
The more I evaluate this |
I missed your comment, and the method is wrong.
|
Something like this will work. |
|
Here is another solution, which you'd need to move some methods back into hosts/ad, so roles/ad can inherit the methods. It doens't make sense on a naming convention, needs to be this way for inheritance. I am fine with either solution. |
3f0ee3b to
70d1157
Compare
| # Install IPA CA certificate so LDAPS/STARTTLS tests can use it without per-test setup. | ||
| # Done before the provisioned check so it runs even on already-provisioned containers. | ||
| cert_pem = ipa.fs.read("/etc/ipa/ca.crt") | ||
| OpenSSLUtils(client, client.fs).install_ca_cert(cert_pem) |
There was a problem hiding this comment.
I see a problem, that there is no check for existing certificates and the cleanup code occurs on topology teardown, which will re-add the certificate to the store on for every test run. Topology teardown only occur when typologies change. We should add a check before issuing install_ca_cert, we can do it here or in install_ca_cert. Up to you.
There was a problem hiding this comment.
updated a check here
There was a problem hiding this comment.
Is this the check? It read the CA certificate on the IPA server before it installs it? We want to check the client so it doesn't have to retrieve the file again.
|
|
||
| return self.install_ca_cert(certs[-1], name=name, cert_path=cert_path) | ||
|
|
||
| def _configure_tls_cacert(self) -> None: |
There was a problem hiding this comment.
This should really be called _configure_openldap they're other certificate stores we may configure.
| ] | ||
| lines.append("SASL_CBINDING tls-endpoint") | ||
|
|
||
| self.fs.write(ldap_conf, "\n".join(lines) + "\n") |
There was a problem hiding this comment.
add fs.backup($PATH) so it resets on teardown.
| ) | ||
| self.fs.chown(homedir, user=user, group=group, args=["-R"]) | ||
|
|
||
| return self.fs.read(f"{homedir}/.ssh/{file}.pub"), self.fs.read(f"{homedir}/.ssh/{file}") |
| # Install IPA CA certificate so LDAPS/STARTTLS tests can use it without per-test setup. | ||
| # Done before the provisioned check so it runs even on already-provisioned containers. | ||
| cert_pem = ipa.fs.read("/etc/ipa/ca.crt") | ||
| OpenSSLUtils(client, client.fs).install_ca_cert(cert_pem) |
There was a problem hiding this comment.
Is this the check? It read the CA certificate on the IPA server before it installs it? We want to check the client so it doesn't have to retrieve the file again.
| # Install CA certificate so LDAPS tests can use it without per-test setup. | ||
| # Done before the provisioned check so it runs even on already-provisioned containers. | ||
| if isinstance(provider, SambaHost): | ||
| ca_cert_path = provider.config.get("ca_cert_path", "/var/data/certs/ca.crt") |
There was a problem hiding this comment.
This also needs a check.
- Is the CA certificate installed on the client, no, get certificate, yes, does the certificate match the provider?
I suggest naming the certificate to match the topology when you initially get them or hash/checksum
| :rtype: str | ||
| :raises RuntimeError: If CA certificate cannot be retrieved. | ||
| """ | ||
| ca_name = self.get_ca_config().split("\\", 1)[1].strip('"') |
There was a problem hiding this comment.
You can simplify this. Since we're already using certutil.
certutil.exe -f -"ca.cert" C:\Windows\Temp\ca.crt
certutil.exe -f -encode C:\Windows\Temp\ca.crt C:\Windows\Temp\ca.pem
Get-Content C:\Windows\Temp\ca.pem -Raw
PS C:\Users\vagrant> certutil.exe -f -"ca.cert" C:\Windows\Temp\ca.crt
CA cert[0]: 3 -- Valid
CA cert[0]:
-----BEGIN CERTIFICATE-----
MIIDVzCCAj+gAwIBAgIQFqpWzzgyZ5ZFdTEfrioBYDANBgkqhkiG9w0BAQsFADA+
MRQwEgYKCZImiZPyLGQBGRYEdGVzdDESMBAGCgmSJomT8ixkARkWAmFkMRIwEAYD
VQQDEwlhZC1Sb290Q0EwHhcNMjYwOTIxMTcxNTM4WhcNMzYwOTIxMTcyNTM3WjA+
MRQwEgYKCZImiZPyLGQBGRYEdGVzdDESMBAGCgmSJomT8ixkARkWAmFkMRIwEAYD
VQQDEwlhZC1Sb290Q0EwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDR
4Tc8etmO+MhnCPvKjuJdNYcJ1Aqo+6ZlNsNH7ueknLkMqRCf4sV4fNj+rQY7CFjF
/ATBaD3yX+cgw6wMuQODmYY2PLkQa7jdNkFmRnF5B7OyiGwznA94uD9LD3ve9nyJ
/kyuY5AWW7D07mExY+5jgmfCh+S5i0OTY0Tn9mRddWpMjvQFn098GIgzhnV1dshW
VtcmKYXWpagaKVy1qa46PEKhIRKfZVUgrl45VxotElE8KFG1GN39IWyT4jTHqCFh
4+VDIUOwz1HQ/MdFR+m48OazQdI5GtFqDbQ5H+600L7Ie1qXuyraMYK3t6xa6OHv
A451Lr+5RXd0X3+MtoEFAgMBAAGjUTBPMAsGA1UdDwQEAwIBhjAPBgNVHRMBAf8E
BTADAQH/MB0GA1UdDgQWBBSpki2czbECFIosuBBTNZdDSqZ/mDAQBgkrBgEEAYI3
FQEEAwIBADANBgkqhkiG9w0BAQsFAAOCAQEADJhXzQVToYEZDvAKu5vmodKQW4oB
eWGaT9AKPHqJ9KgWmfWXaNmm/eNQidfKkuegYYfFTtUuZC9NaTTcAjSzSs+C8eiu
e9reh3NZRTDaY/64NZovyvZUL0XturB7SQGr79RGzekDu4r6zzD810zmJw4EkdgA
w8z4mckcFxZDHgcY8oD6jhCHOaYWs7knClIZ0euu+DGExeQ+OAIA/cfgUxLMNqjo
nF0f/m69M3iqk7JrKQXJJgAZfRmoV1nuCftMF73IlGkmUjT2yHsnn8nJdXDZDP1f
SVUgAH5ckb1sNrlTqpnjFkCL1CrtqRNNlwDLq+mT+Vfz3VCCjS7oOvFgkw==
-----END CERTIFICATE-----
CertUtil: -ca.cert command completed successfully.
PS C:\Users\vagrant> certutil.exe -f -encode C:\Windows\Temp\ca.crt C:\Windows\Temp\ca.pem
Input Length = 859
Output Length = 1240
CertUtil: -encode command completed successfully.
PS C:\Users\vagrant> Get-Content C:\Windows\Temp\ca.pem -Raw
-----BEGIN CERTIFICATE-----
MIIDVzCCAj+gAwIBAgIQFqpWzzgyZ5ZFdTEfrioBYDANBgkqhkiG9w0BAQsFADA+
MRQwEgYKCZImiZPyLGQBGRYEdGVzdDESMBAGCgmSJomT8ixkARkWAmFkMRIwEAYD
VQQDEwlhZC1Sb290Q0EwHhcNMjYwOTIxMTcxNTM4WhcNMzYwOTIxMTcyNTM3WjA+
MRQwEgYKCZImiZPyLGQBGRYEdGVzdDESMBAGCgmSJomT8ixkARkWAmFkMRIwEAYD
VQQDEwlhZC1Sb290Q0EwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDR
4Tc8etmO+MhnCPvKjuJdNYcJ1Aqo+6ZlNsNH7ueknLkMqRCf4sV4fNj+rQY7CFjF
/ATBaD3yX+cgw6wMuQODmYY2PLkQa7jdNkFmRnF5B7OyiGwznA94uD9LD3ve9nyJ
/kyuY5AWW7D07mExY+5jgmfCh+S5i0OTY0Tn9mRddWpMjvQFn098GIgzhnV1dshW
VtcmKYXWpagaKVy1qa46PEKhIRKfZVUgrl45VxotElE8KFG1GN39IWyT4jTHqCFh
4+VDIUOwz1HQ/MdFR+m48OazQdI5GtFqDbQ5H+600L7Ie1qXuyraMYK3t6xa6OHv
A451Lr+5RXd0X3+MtoEFAgMBAAGjUTBPMAsGA1UdDwQEAwIBhjAPBgNVHRMBAf8E
BTADAQH/MB0GA1UdDgQWBBSpki2czbECFIosuBBTNZdDSqZ/mDAQBgkrBgEEAYI3
FQEEAwIBADANBgkqhkiG9w0BAQsFAAOCAQEADJhXzQVToYEZDvAKu5vmodKQW4oB
eWGaT9AKPHqJ9KgWmfWXaNmm/eNQidfKkuegYYfFTtUuZC9NaTTcAjSzSs+C8eiu
e9reh3NZRTDaY/64NZovyvZUL0XturB7SQGr79RGzekDu4r6zzD810zmJw4EkdgA
w8z4mckcFxZDHgcY8oD6jhCHOaYWs7knClIZ0euu+DGExeQ+OAIA/cfgUxLMNqjo
nF0f/m69M3iqk7JrKQXJJgAZfRmoV1nuCftMF73IlGkmUjT2yHsnn8nJdXDZDP1f
SVUgAH5ckb1sNrlTqpnjFkCL1CrtqRNNlwDLq+mT+Vfz3VCCjS7oOvFgkw==
-----END CERTIFICATE-----
| .. code-block:: python | ||
| :caption: Example usage | ||
|
|
||
| @pytest.mark.topology(KnownTopologyGroup.AnyDC) |
There was a problem hiding this comment.
We should use actual tests for examples, because they'll be generated sphinx docs.
| .. code-block:: python | ||
| :caption: Example usage | ||
|
|
||
| @pytest.mark.topology(KnownTopologyGroup.AnyDC) |
There was a problem hiding this comment.
Use a real example, please.
c31b96c to
e2a23db
Compare
|
FYI, latest updates are passing: |
spoore1
left a comment
There was a problem hiding this comment.
Small change suggestion unless I'm misreading something.
060ea99 to
a279b3d
Compare
|
Hi, I'm still fine with this PR, but I think ti would be good to squash some of the related patches into one. bye, |
Add export_root_ca_certificate() abstract method to GenericProvider and implement it in the AD, IPA, and Samba roles. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add OpenSSLUtils to utils/tools.py with install_ca_cert(), install_ca_cert_from_server(), and _configure_openldap() helpers. Add install_ca_cert() and ssl attribute to the Client role. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ADTopologyController (Samba) and IPATopologyController now install the provider's CA certificate on the client via OpenSSLUtils before the provisioned check, so LDAPS/STARTTLS tests have the cert available without any per-test setup. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace use_ldaps() with ssl_tls() in SSSDCommonConfiguration. The CA certificate is now pre-installed by the topology controller, so ssl_tls() only sets the SSSD domain options (ad_use_ldaps or ldap_id_use_start_tls) and ldap_tls_cacert path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove install_ca_cert(), install_ca_cert_from_server(), use_ldaps(), and _configure_tls_cacert() from the Client role. Add self.ssl (OpenSSLUtils) for tests that still need to install a cert manually (e.g. AD with no AD CS). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Move cert export logic from ADCertificateAuthority to ADHost so topology controllers can access it directly without going through role fixtures. ADCertificateAuthority._get_ca_config() and get_ca_cert() now delegate to self.host, keeping the role-level API unchanged. ADTopologyController gains an ADHost cert install branch with two-level fallback: ADCS/PowerShell first, then openssl s_client, so cert install works regardless of whether Windows Certificate Services is present. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
a279b3d to
990773a
Compare
Skip the write and update-ca-trust call if the certificate file already exists with identical content. This avoids redundant re-installation on every topology setup cycle when using provisioned containers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
990773a to
d61eee7
Compare
spoore1
left a comment
There was a problem hiding this comment.
Just two minor questions about using the cat command.
Code appears to be working and I ran it with a slight modification to the tests in SSSD/sssd#9137
| ldap_conf = "/etc/openldap/ldap.conf" | ||
|
|
||
| self.fs.backup(ldap_conf) | ||
| result = self.host.conn.run(f"cat {ldap_conf}", raise_on_error=False) |
There was a problem hiding this comment.
Is there a reason here for running the cat command instead of using self.fs.read()?
There was a problem hiding this comment.
updated to self.fs.read()
pls check now
| if isinstance(provider, SambaHost): | ||
| if not client.fs.exists("/etc/pki/ca-trust/source/anchors/samba-ca.crt"): | ||
| ca_cert_path = provider.config.get("ca_cert_path", "/var/data/certs/ca.crt") | ||
| result = provider.conn.run(f"cat {ca_cert_path}", raise_on_error=False) |
There was a problem hiding this comment.
Is there a reason here for cat instead of self.fs.read()?
There was a problem hiding this comment.
updated to self.fs.read()
- topology_controllers: check client before fetching cert from provider to skip redundant SSH round-trips on re-provisioned containers; name certs by topology (ipa-ca.crt, samba-ca.crt, ad-ca.crt); replace conn.run cat with fs.exists/fs.read - utils/tools: add fs.backup() for /etc/openldap/ldap.conf so it is restored on teardown; replace conn.run cat with fs.exists/fs.read - utils/sssd: ssl_tls() derives default cacert path from provider type Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>